-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove ambiguous fields from AuditEntry #3017
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3017 +/- ##
==========================================
- Coverage 97.88% 97.71% -0.17%
==========================================
Files 151 151
Lines 13024 13072 +48
==========================================
+ Hits 12748 12773 +25
- Misses 196 211 +15
- Partials 80 88 +8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @WillAbides !
Just a few nits, please, then we will be ready for a second LGTM+Approval from any other contributor to this repo before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @WillAbides !
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you @valbeat ! |
Sorry, I'm going to have to work on this one because of all the other recent merges. Hopefully tomorrow I can work on it. |
Signed-off-by: Glenn Lewis <[email protected]>
Signed-off-by: Glenn Lewis <[email protected]>
Note that the new |
closes #3016
In #3016 (comment) I suggested that AuditEntry fields should be limited to what is in the openapi description. I have since spoken with some people at GitHub and learned that the openapi description is outdated and there are discrepancies between the description and the json being returned by the api.
Instead of limiting AuditEntry to what is in the openapi description, this PR reduces AuditEntry to fields that are known to be the same type across all events regardless of the openapi description. The rest of the fields are now available in
AuditEntry.AdditionalFields
as amap[string]any
This is a breaking change.